Skip to content
New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

4396 packs 1 add custom request units at organizational level #4415

Conversation

Eccatoe
Copy link
Collaborator

@Eccatoe Eccatoe commented Jun 1, 2024

Resolves #4396

@cielf
Copy link
Collaborator

cielf commented Jun 1, 2024

Hey @Eccatoe -- Thanks for this!

I'm doing some manual testing.

  1. I notice that the changes are not behind the flipper flag "enable_packs" as noted in the issue.

@cielf
Copy link
Collaborator

cielf commented Jun 1, 2024

  1. Noticed that the lint is failing.

@cielf
Copy link
Collaborator

cielf commented Jun 1, 2024

  1. Please change the "Add another item" button, here to "Add another unit"
    Screenshot 2024-06-01 at 2 06 47 PM

@cielf
Copy link
Collaborator

cielf commented Jun 1, 2024

  1. This one is a little more complicated...
    If I fill in the field for the first unit, click "Add another item", then click "Remove" on the second line, the first unit disappears.

@cielf
Copy link
Collaborator

cielf commented Jun 1, 2024

  1. I added a unit and saved it, then went back in, added a second unit, and saved.
    I got this:
Screenshot 2024-06-01 at 2 18 25 PM

Copy link
Collaborator

@cielf cielf left a comment

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Hey @Eccatoe - Good start! I was assuming this was ready for review (because it's Open rather than Draft). If thats not the case, please just treat the comments as a check-list for finishing it up.
This is a first functional review, rather than a code review per se. If you have any problem with recreating, please ping me on slack and I can walk you through it.

@Eccatoe
Copy link
Collaborator Author

Eccatoe commented Jun 1, 2024

@cielf Sorry, I should have tagged this as a Draft, it is certainly a WIP. Thank you for the feedback, we will work on those issues!

@@ -15,7 +15,8 @@ def edit
def update
@organization = current_organization

if OrganizationUpdateService.update(@organization, organization_params)
@units = Unit.where(params.dig(:request_units_attributes)&.dig(:name))
Copy link
Collaborator Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

TODO deleting all units - needs fix

@awwaiid awwaiid marked this pull request as draft June 2, 2024 12:46
@awwaiid
Copy link
Collaborator

awwaiid commented Jun 22, 2024

This PR was continued over at #4432! Thank you!

@awwaiid awwaiid closed this Jun 22, 2024
@awwaiid awwaiid added this to the Request Units (Packs) milestone Jul 21, 2024
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
None yet
Projects
None yet
Development

Successfully merging this pull request may close these issues.

[PACKS] # 1 Add custom request units at organizational level
4 participants